Skip to content

Fix join precedence for non-snowflake queries #1905

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 28, 2025
Merged

Conversation

Dimchikkk
Copy link
Contributor

Fixes #1904.

The bug was introduced in #1799.

Copy link
Contributor

@iffyio iffyio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix @Dimchikkk! Left a couple comments

@@ -12486,7 +12486,7 @@ impl<'a> Parser<'a> {
};
let mut relation = self.parse_table_factor()?;

if self.peek_parens_less_nested_join() {
if dialect_of!(self is SnowflakeDialect) && self.peek_parens_less_nested_join() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we change this to a dialect method that is only set to true for snowflake? e.g. if self.dialect.supports_parens_less_nested_join() && self.peek_parens_less_nested_join()

@@ -15357,6 +15357,28 @@ fn check_enforced() {
);
}

#[test]
fn join_precedence() {
all_dialects_except(|d| d.is::<SnowflakeDialect>()).verified_query_with_canonical(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By switching to the dialect method, we would be able to change these to use all_dialects_where(|d|d.supports_parens_less_nested_join())

@Dimchikkk
Copy link
Contributor Author

Thanks for the review, @iffyio - I've addressed your feedback and it’s ready for another round when you are.

Copy link
Contributor

@iffyio iffyio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks again @Dimchikkk!
cc @alamb

@@ -278,6 +278,34 @@ pub trait Dialect: Debug + Any {
false
}

/// Indicates whether the dialect supports left-associative join parsing
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice thanks for the description!

@iffyio iffyio merged commit 3bc9423 into apache:main Jun 28, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong join precedence parsing for non-Snowflake dialects (nested joins parsed incorrectly)
2 participants